Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plane: Quadplane: add option to refuse change to FW mode at low altitude #21289

Merged
merged 1 commit into from
Aug 23, 2022

Conversation

IamPete1
Copy link
Member

@IamPete1 IamPete1 commented Jul 27, 2022

This is now a script that will change modes to either Qland or back to previous mode is a FW mode is switched into at low altitude within set radius of home.

@Hwurzburg Hwurzburg added the WikiNeeded needs wiki update label Jul 27, 2022
@Hwurzburg Hwurzburg self-requested a review July 27, 2022 19:41
Copy link
Collaborator

@Hwurzburg Hwurzburg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens in failsafe(S)? should accept RTL mode change

@IamPete1
Copy link
Member Author

IamPete1 commented Jul 27, 2022

There are lots of options in the fail safe case. Currently this will refuse the mode change. Which may be the right thing to do if too low to transition. We could make the current code smarter in that case, so if change to RTL fails then try QRTL or QLAND. Or we could only block pilot mode changes from either RC or GCS and let everything else work as now.

I think RTL is the only mode you might want to get into and not be blocked, so we could also special case RTL mode. Depending on your QRTL mode param it might not try and fly in fixed wing at all.

return true;
}
int32_t alt_cm;
if (!plane.current_loc.initialised() || !plane.current_loc.get_alt_cm(Location::AltFrame::ABOVE_HOME, alt_cm)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could use the height above ground estimate from the rangefinder here. But then were using a different height reference than fence applys to its min altitude.

@Hwurzburg
Copy link
Collaborator

Hwurzburg commented Jul 28, 2022

Personally, I am not a fan of how this is setup....making one have to ascend to a typically high FENCE or FBWB_MIN is too much of a burden...I would typically set those at least above surrounding area treeline (maybe 100feet or more), where one might only need an altitude of 20-30 feet to safely transition, and climb out.....since you are not tackling the more important cases of where a pilot has no immediate control , ie AUTO, and actually denying the pilot the choice of how high to transition based on params setup for a wider area of operation, I feel this unnecessarily restricts pilot control...and not allowing a FS to go to one of its designated modes is very bad in my opinion...especially since normally ALL pilot control is lost in the failsafe due to link loss, denying RTL FS mode change is dooming the vehicle

aside from the FS issue, if this was a new independent param to pre-arm warn of VTOL takeoffs to some minimum safe transition alt, that might have value, but then again, if the user cant set a proper alt for VTOL takeoffs, why would we believe he would set the new param?

@peterbarker
Copy link
Contributor

Maybe split out the enum change into another PR?

I'd also suggest option_enabled(...) or option_is_set(...)rather than option_set as the latter could be read as "set this option"

@IamPete1
Copy link
Member Author

Since the conclusion was that is was better to switch to QLand than refuse the mode change I realized we could just do it in scripting.

@IamPete1
Copy link
Member Author

This still does not really have a great way to deal with failsafe cases because scripting has no way to tell. We could add bindings for that, or mode reason.

@tridge tridge merged commit 08b458c into ArduPilot:master Aug 23, 2022
@LupusTheCanine
Copy link

In case of RTL the script could ascend then enable RTL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants